docs(design): gateway-credentials at-rest encryption + two-layer secret providers (RIG-2863) - #859
Open
rigel-mintaka wants to merge 5 commits into
Open
docs(design): gateway-credentials at-rest encryption + two-layer secret providers (RIG-2863)#859rigel-mintaka wants to merge 5 commits into
rigel-mintaka wants to merge 5 commits into
Conversation
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
Compass engineering docs preview: https://compass-server-rig-2863-cred.compass-eng-docs.pages.dev Deployed from Changed pages: |
rigel-mintaka
added a commit
that referenced
this pull request
Sep 4, 2026
…-2863) Round-1 review of #859 found 2 high + 4 medium, all folded (no Matt ruling relitigated): - **F1 (high)** — corrected the false "user SetSecret path physically CANNOT reach the master key" claim: C1 splits the declaration registries, NOT the shared SecretSpec provider keyspace, so the reserved-name guard is now mandatory on BOTH the admin RPC and the authenticatedOpen user SetSecret/DeleteSecret path. Added the user-path red-green test and a boot key-fingerprint tripwire (T5) so a swapped provider value is detected, not silently adopted. - **F2 (high)** — replaced T0's non-executable in-migration DML row-move with a boot-time config-driven reconcile (server-secret names are per-deployment flag/env config the embedded migration cannot know), and enumerated all six names (primary + reviewer App PEM, webhook, three Linear secrets). - **F3 (medium)** — added the mandatory GRANT to compass_app/compass_system (0001's ON ALL TABLES snapshot does not cover later migrations) and a stated bucket-A RLS decision (Matt-ruled: server_secrets is deployment-global infra, no tenant_id, RLS off, added to the rls_pgtest bucketA allow-list). - **F4 (medium)** — OQ-3 resolved to D7 (Matt: api_key rows encrypted identically to OAuth rows, one seal/open path). - **F5 (medium)** — re-anchored four drifted 0001_init.sql citations (post-RIG-3106 line shift). - **F6 (medium)** — D1 column names aligned to T3/ledger (value_ciphertext/value_nonce). - **F7/F8 (low)** — added the RIG-2863 tracker ref; named the C2 residual risk concretely. Follow-up RIG-3237 filed for per-tenant at-rest isolation + gateway-topology exposure. Spec-impact: none (design record). Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 4, 2026
…-2863) Round-1 review of #859 found 2 high + 4 medium, all folded (no Matt ruling relitigated): - **F1 (high)** — corrected the false "user SetSecret path physically CANNOT reach the master key" claim: C1 splits the declaration registries, NOT the shared SecretSpec provider keyspace, so the reserved-name guard is now mandatory on BOTH the admin RPC and the authenticatedOpen user SetSecret/DeleteSecret path. Added the user-path red-green test and a boot key-fingerprint tripwire (T5) so a swapped provider value is detected, not silently adopted. - **F2 (high)** — replaced T0's non-executable in-migration DML row-move with a boot-time config-driven reconcile (server-secret names are per-deployment flag/env config the embedded migration cannot know), and enumerated all six names (primary + reviewer App PEM, webhook, three Linear secrets). - **F3 (medium)** — added the mandatory GRANT to compass_app/compass_system (0001's ON ALL TABLES snapshot does not cover later migrations) and a stated bucket-A RLS decision (Matt-ruled: server_secrets is deployment-global infra, no tenant_id, RLS off, added to the rls_pgtest bucketA allow-list). - **F4 (medium)** — OQ-3 resolved to D7 (Matt: api_key rows encrypted identically to OAuth rows, one seal/open path). - **F5 (medium)** — re-anchored four drifted 0001_init.sql citations (post-RIG-3106 line shift). - **F6 (medium)** — D1 column names aligned to T3/ledger (value_ciphertext/value_nonce). - **F7/F8 (low)** — added the RIG-2863 tracker ref; named the C2 residual risk concretely. Follow-up RIG-3237 filed for per-tenant at-rest isolation + gateway-topology exposure. Spec-impact: none (design record). Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 4, 2026
…(RIG-2863) Round-2 review of #859 returned 0 high / 3 medium / 3 low. The round-1 body fixes all held; the blockers were that the fix commit refreshed the record body but not the terse `## Tasks` checklist, plus one new defect the F2 fix introduced. - R2-1/R2-2 (medium): rewrite the T0 + T2 `## Tasks` checklist entries to match the fixed body — T2 no longer says the reserved-name guard is admin-RPC-only, and T0 now names the boot-time config-driven reconcile (not in-migration DML), all six server-secret names, the GRANT, and the bucketA allow-list edit. - R2-3 (medium, new defect): the F2 fix relocated the row move from a pre-policy migration to a post-policy boot reconcile, where the source `secrets` table is FORCE RLS and the ordinary tenant-scoped store path sees only the bootstrap tenant. Specify the reconcile runs under `store.WithSystemRole` (BYPASSRLS) — a fifth WithSystemRole entrypoint that extends DL-315's "only at the four named entrypoints" clause — and add the two-tenant reconcile red-green test. - R2-4 (low): add the key-swap fingerprint tripwire to the T5 checklist entry. - R2-5/R2-6 (low): correct two citation anchors added by the round-1 fixes (`rls_pgtest_test.go:544`->`:543`; `secrets_service.go:214`->`:208`). Additive review-fix commit atop the round-1 fix; no ledger delta (DL-324 is unchanged — the reconcile execution context is a T0 mechanism detail, ratified at the PR gate). Ledger-impact: none Spec-impact: none Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
force-pushed
the
compass-server/rig-2863-creds-encryption-design
branch
from
September 4, 2026 18:04
58b972e to
d62e9ad
Compare
rigel-mintaka
added a commit
that referenced
this pull request
Sep 4, 2026
…-2863) Round-1 review of #859 found 2 high + 4 medium, all folded (no Matt ruling relitigated): - **F1 (high)** — corrected the false "user SetSecret path physically CANNOT reach the master key" claim: C1 splits the declaration registries, NOT the shared SecretSpec provider keyspace, so the reserved-name guard is now mandatory on BOTH the admin RPC and the authenticatedOpen user SetSecret/DeleteSecret path. Added the user-path red-green test and a boot key-fingerprint tripwire (T5) so a swapped provider value is detected, not silently adopted. - **F2 (high)** — replaced T0's non-executable in-migration DML row-move with a boot-time config-driven reconcile (server-secret names are per-deployment flag/env config the embedded migration cannot know), and enumerated all six names (primary + reviewer App PEM, webhook, three Linear secrets). - **F3 (medium)** — added the mandatory GRANT to compass_app/compass_system (0001's ON ALL TABLES snapshot does not cover later migrations) and a stated bucket-A RLS decision (Matt-ruled: server_secrets is deployment-global infra, no tenant_id, RLS off, added to the rls_pgtest bucketA allow-list). - **F4 (medium)** — OQ-3 resolved to D7 (Matt: api_key rows encrypted identically to OAuth rows, one seal/open path). - **F5 (medium)** — re-anchored four drifted 0001_init.sql citations (post-RIG-3106 line shift). - **F6 (medium)** — D1 column names aligned to T3/ledger (value_ciphertext/value_nonce). - **F7/F8 (low)** — added the RIG-2863 tracker ref; named the C2 residual risk concretely. Follow-up RIG-3237 filed for per-tenant at-rest isolation + gateway-topology exposure. Spec-impact: none (design record). Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 4, 2026
…(RIG-2863) Round-2 review of #859 returned 0 high / 3 medium / 3 low. The round-1 body fixes all held; the blockers were that the fix commit refreshed the record body but not the terse `## Tasks` checklist, plus one new defect the F2 fix introduced. - R2-1/R2-2 (medium): rewrite the T0 + T2 `## Tasks` checklist entries to match the fixed body — T2 no longer says the reserved-name guard is admin-RPC-only, and T0 now names the boot-time config-driven reconcile (not in-migration DML), all six server-secret names, the GRANT, and the bucketA allow-list edit. - R2-3 (medium, new defect): the F2 fix relocated the row move from a pre-policy migration to a post-policy boot reconcile, where the source `secrets` table is FORCE RLS and the ordinary tenant-scoped store path sees only the bootstrap tenant. Specify the reconcile runs under `store.WithSystemRole` (BYPASSRLS) — a fifth WithSystemRole entrypoint that extends DL-315's "only at the four named entrypoints" clause — and add the two-tenant reconcile red-green test. - R2-4 (low): add the key-swap fingerprint tripwire to the T5 checklist entry. - R2-5/R2-6 (low): correct two citation anchors added by the round-1 fixes (`rls_pgtest_test.go:544`->`:543`; `secrets_service.go:214`->`:208`). Additive review-fix commit atop the round-1 fix; no ledger delta (DL-324 is unchanged — the reconcile execution context is a T0 mechanism detail, ratified at the PR gate). Ledger-impact: none Spec-impact: none Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
force-pushed
the
compass-server/rig-2863-creds-encryption-design
branch
from
September 4, 2026 19:27
d62e9ad to
d6f2eee
Compare
rigel-mintaka
added a commit
that referenced
this pull request
Sep 4, 2026
…863) Review of #868 returned 0 high / 1 medium / 2 low. Citations (16/16) and the SubjectService ledger row were a clean pass; the blocker was executor-contract coherence. - F1 (medium): the Plan framed all five tasks as one "PR2 executor contract", but T4 (the service-door mount) targets a Server surface that does not exist in the tree yet, and no corpus task owns minting a SubjectService token — so the class as written ships unreachable. Reframed the Plan into two scopes: the enum half (T1/T2/T3/T5) lands now as PR2 (the urgent half — enum number + CHECK are painful to change once token rows exist); the door half (T4) lands WITH the RIG-2863 (RIG-1715 T2) service surface it mounts on, ordered after it. Added OQ-3 naming where token issuance lives (the RIG-2863 T4 slice, an IssueServiceToken mirroring MintRunnerToken) so an executor never improvises a mint path on a security-critical door. - F2 (low): citation span `lines 348-350` -> `348-351` (the quoted "disable)" falls on 351). - F3 (low, pre-existing): noted that `runnerhub.RunnerTokenRegistered` is the one token-store consumer outside the kind-gated door contract — flagged for the T4 executor to kind-check or document, out of scope for this record. Ledger-collision renumber: main's #870 (RIG-3096) merged its own DL-324 (forge live-test app-actor) after this stack forked, and the sibling encryption PR (#859) took DL-325/DL-326. Renumbered this record's SubjectService row to the next free id, DL-327 (was DL-325), leaving main's DL-324 untouched. Additive review-fix commit. Ledger-impact: DL-327 (SubjectService, renumbered from DL-325) Spec-impact: none Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 4, 2026
…sign (RIG-3238) ReviewDesign869r2 verified all 7 round-1 findings CLOSED but caught two mediums the round-1 fold's own new prose introduced. Both are mechanical text corrections with no design consequence. - MEDIUM (guard-bullet facts): the ID-allocation/freeze-order Global Constraint bullet had wrong PR->DL attributions on every clause. Ground truth (verified via gh pr diff + main's ledger): main's tail ends at DL-324 (landed by #870, merged); DL-325 is claimed by #804 (RIG-3070, draft) AND #859 (RIG-2863); DL-326 -- this record's own number -- is claimed by #859, which is review-ready (NOT draft). The bullet missed the real live collision (#859 on DL-326) and named #868 (which actually claims DL-327). Rewrote the factual clauses to match; kept the two correct load-bearing clauses (the MUST-re-grep-before-landing instruction and does-NOT-depend-on-#804). The immutable DL-326 cell was already merge-order-safe (name+issue cite), so this only corrected mutable prose. - MEDIUM (unknown-count consistency): round-1 widened the ledger cell from five to six unknowns (adding runner-on-darwin / OQ-12) but left three "five" sites in the record body: the Why-STAGED enumeration, the reject-default alternative, and the T-1 task heading. Aligned all three to six + OQ-12, and aligned the threat classification (OQ-1, OQ-2, and OQ-12 threaten adoption) with the cell's widened re-open trigger. - LOW: re-wrapped two range-overwrite orphan lines to the file's ~72-col width. Checks: markdownlint 0 errors; design-ledger-gate OK (291 rows, 122 headers). Status stays Draft -- freezes Active on merge. Refs RIG-3238 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 4, 2026
…-2863) Round-1 review of #859 found 2 high + 4 medium, all folded (no Matt ruling relitigated): - **F1 (high)** — corrected the false "user SetSecret path physically CANNOT reach the master key" claim: C1 splits the declaration registries, NOT the shared SecretSpec provider keyspace, so the reserved-name guard is now mandatory on BOTH the admin RPC and the authenticatedOpen user SetSecret/DeleteSecret path. Added the user-path red-green test and a boot key-fingerprint tripwire (T5) so a swapped provider value is detected, not silently adopted. - **F2 (high)** — replaced T0's non-executable in-migration DML row-move with a boot-time config-driven reconcile (server-secret names are per-deployment flag/env config the embedded migration cannot know), and enumerated all six names (primary + reviewer App PEM, webhook, three Linear secrets). - **F3 (medium)** — added the mandatory GRANT to compass_app/compass_system (0001's ON ALL TABLES snapshot does not cover later migrations) and a stated bucket-A RLS decision (Matt-ruled: server_secrets is deployment-global infra, no tenant_id, RLS off, added to the rls_pgtest bucketA allow-list). - **F4 (medium)** — OQ-3 resolved to D7 (Matt: api_key rows encrypted identically to OAuth rows, one seal/open path). - **F5 (medium)** — re-anchored four drifted 0001_init.sql citations (post-RIG-3106 line shift). - **F6 (medium)** — D1 column names aligned to T3/ledger (value_ciphertext/value_nonce). - **F7/F8 (low)** — added the RIG-2863 tracker ref; named the C2 residual risk concretely. Follow-up RIG-3237 filed for per-tenant at-rest isolation + gateway-topology exposure. Spec-impact: none (design record). Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 4, 2026
…(RIG-2863) Round-2 review of #859 returned 0 high / 3 medium / 3 low. The round-1 body fixes all held; the blockers were that the fix commit refreshed the record body but not the terse `## Tasks` checklist, plus one new defect the F2 fix introduced. - R2-1/R2-2 (medium): rewrite the T0 + T2 `## Tasks` checklist entries to match the fixed body — T2 no longer says the reserved-name guard is admin-RPC-only, and T0 now names the boot-time config-driven reconcile (not in-migration DML), all six server-secret names, the GRANT, and the bucketA allow-list edit. - R2-3 (medium, new defect): the F2 fix relocated the row move from a pre-policy migration to a post-policy boot reconcile, where the source `secrets` table is FORCE RLS and the ordinary tenant-scoped store path sees only the bootstrap tenant. Specify the reconcile runs under `store.WithSystemRole` (BYPASSRLS) — a fifth WithSystemRole entrypoint that extends DL-315's "only at the four named entrypoints" clause — and add the two-tenant reconcile red-green test. - R2-4 (low): add the key-swap fingerprint tripwire to the T5 checklist entry. - R2-5/R2-6 (low): correct two citation anchors added by the round-1 fixes (`rls_pgtest_test.go:544`->`:543`; `secrets_service.go:214`->`:208`). Additive review-fix commit atop the round-1 fix; no ledger delta (DL-324 is unchanged — the reconcile execution context is a T0 mechanism detail, ratified at the PR gate). Ledger-impact: none Spec-impact: none Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
force-pushed
the
compass-server/rig-2863-creds-encryption-design
branch
from
September 4, 2026 21:08
9ee2a9e to
899aabc
Compare
rigel-mintaka
added a commit
that referenced
this pull request
Sep 4, 2026
…(RIG-2863) Round-5 adversarial review of PR #859 returned 2H/3M/2L; all seven verified against the tree at main `8fef2846` before folding. None is a design fork — each corrects a plan-executability or internal-consistency defect that honors an invariant the record already asserts. Additive commit atop the bookmark. - **H1 (silent forge-write capability loss):** the T0 failure-mode taxonomy was two modes ((a) hard boot fail, (b) silent Linear degradation) but a THIRD exists that the reviewer-PEM `validateForgeSecret` (serve.go:1638) hides behind. `wireForgeWriteCaller` (:587) gates on `forgeWritesEnabled` (:1583); with both App PEM names moved out of the container resolver the predicate reads both-absent, `warnPartialForgeWriteSecrets` returns silently (:1826-1827), the write caller is never mounted, and :1638 is unreachable — agent forge WRITES fail-closed fleet-wide with no boot error. Split the taxonomy into three modes, moved :1638 into mode (c), and added the missing T0 positive assertion (forge WRITE caller MOUNTED after one reconcile pass). - **H2 (uncancellable Resolve wedge):** the T2 bounded-critical-section fix prescribed a ctx timeout, but `SpecResolver.Resolve` threads ctx only into `DeclaredSecrets` (resolver.go:136); the provider round-trip is `b.Load()` (resolver.go:165), whose SDK signature carries no ctx (secretspec-go v0.15.0 secretspec.go:245) and blocks in an uncancellable FFI call. State the Set(bounded)/Resolve(unbounded) asymmetry and prescribe the mechanism that works: run the boot resolve on its own goroutine + select on the derived ctx, so the provisioner returns a bounded error and releases the xact lock while the orphaned FFI goroutine is knowingly leaked. Restated the T2 test. - **M1 (reconcile skips default-named Linear pair):** pinned the reconcile name source to `cfg.Forge.resolved()` (serve.go:232-246), not the raw flag/env layer — two Linear names carry code defaults (serve.go:202-203) applied after that layer, so a reconcile over `main.go` flags sees `""` and silently skips them, leaving them on the inject-all path. Added a T0 test. - **M2 (buildDoors resolver-fork hazard):** named the `buildDoors` fork explicitly — its single `resolver` param (serve.go:677) feeds both `buildLinearWebhookWiring` (:788, MUST repoint) and `buildNetworkServer` (:803) -> `runnerhub.NewMountedHandler` (network_door.go:313, the container FetchSecrets path that MUST NOT). So `buildDoors` takes BOTH resolvers; swapping its one argument would invert D6. - **M3 (dangling D7 reference):** DL-325 and OQ-3 cited a decision "D7" the record never defined. Promoted the api_key/OAuth uniform-treatment ruling into a real `### D7` heading in the Approach section; OQ-3 and DL-325 now resolve. - **L1:** defined the reserved prefix `GATEWAY_CREDENTIALS_` inline at its three uses (was undefined). - **L2:** caveated the header "structurally undeliverable" claim with the F1 guard dependency the D6 body already states (the split alone is necessary but not sufficient). markdownlint clean; design-ledger-gate OK (293 rows, 123 headers); no dup DL ids. Record grew to 59547 B; DL-325/DL-326 anchors already present. Ledger-impact: DL-325, DL-326 (text unchanged; D7 referent now exists) Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 4, 2026
…863) Review of #868 returned 0 high / 1 medium / 2 low. Citations (16/16) and the SubjectService ledger row were a clean pass; the blocker was executor-contract coherence. - F1 (medium): the Plan framed all five tasks as one "PR2 executor contract", but T4 (the service-door mount) targets a Server surface that does not exist in the tree yet, and no corpus task owns minting a SubjectService token — so the class as written ships unreachable. Reframed the Plan into two scopes: the enum half (T1/T2/T3/T5) lands now as PR2 (the urgent half — enum number + CHECK are painful to change once token rows exist); the door half (T4) lands WITH the RIG-2863 (RIG-1715 T2) service surface it mounts on, ordered after it. Added OQ-3 naming where token issuance lives (the RIG-2863 T4 slice, an IssueServiceToken mirroring MintRunnerToken) so an executor never improvises a mint path on a security-critical door. - F2 (low): citation span `lines 348-350` -> `348-351` (the quoted "disable)" falls on 351). - F3 (low, pre-existing): noted that `runnerhub.RunnerTokenRegistered` is the one token-store consumer outside the kind-gated door contract — flagged for the T4 executor to kind-check or document, out of scope for this record. Ledger-collision renumber: main's #870 (RIG-3096) merged its own DL-324 (forge live-test app-actor) after this stack forked, and the sibling encryption PR (#859) took DL-325/DL-326. Renumbered this record's SubjectService row to the next free id, DL-327 (was DL-325), leaving main's DL-324 untouched. Additive review-fix commit. Ledger-impact: DL-327 (SubjectService, renumbered from DL-325) Spec-impact: none Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 4, 2026
…(RIG-2863) Round-7 review returned 0 high / 4 medium / 3 low on PR #859; all seven grounded against the tree at `eb5ef7a1` and folded additively. ### Mediums - **M1 — key-swap tripwire had no schema home.** T5 placed the non-secret key fingerprint "beside `key_version` in `server_secrets`", but `key_version` is a T3 column on `gateway_credentials` and `server_secrets` is names-only — a circular, unimplementable reference for the T0-then-T2 executor. Gave the tripwire a real home: a single-row `server_key_state` table (`key_version`/`key_fingerprint`/`fingerprint_salt`) in T0's migration, bucket-A infrastructure joining the RLS allow-list; a salted digest is not a VALUE, so the names-only invariant is untouched. Rewrote the T2 pointer and T5 placement to name it; added it to the T0 Tasks line. - **M2 — superuser citation inverted.** The record claimed `rls_pgtest_test.go:282-284` asserts the superuser-owner property "not of production", but the cited comment reads "the pgtest harness (like production) connects as a SUPERUSER owner" — the opposite — and it is a prose comment, not an assertion. Rewrote the parenthetical to match the cited text (assumption stated in two comments, not an enforced invariant). - **M3 — bounded-critical-section self-contradiction.** One sentence said "runs the boot RESOLVE/SET on its own goroutine" while the same paragraph establishes `Set` is ctx-bounded (`exec.CommandContext`, resolver.go:233) and "offloads ONLY the ctx-less provider call". Under the RESOLVE/SET reading a leaked goroutine could `Set` after the parent's Rollback, outside the released advisory lock, overwriting a key another booter already sealed rows under — the exact race the lock exists to close. Made it unambiguous: only the ctx-less `Resolve` is offloaded; the parent runs `Set` in-lock on the success branch; the offloaded goroutine performs no provider write. - **M4 — F1 membership guard one-directional.** The user path rejected a `server_secrets` name, but `SetServerSecret` carried no mirror guard against a name already live in `secrets`, and the reconcile skipped a both-tables name — the same D6-inverting end state, reachable in the opposite order and unhealed. Restated the invariant order-free ("no name ever in both tables"), made F1 symmetric (mirror guard on the admin RPC), made the reconcile self-healing (deletes the `secrets` shadow row on collision), and added the two red-green tests. ### Lows - **L1 (DECISIONS.md DL-325)** — named the symmetric F1 membership guard alongside the reserved-name guard (the reserved-name check covers only 1 of 7 names). - **L2** — added an ordering note that failure mode (c) is masked by (a) on a full no-repoint and is the partial-repoint mode. - **L3** — split the mode-(b) citation to its two real sites (`forgeSecretDeclared` at serve.go:1746-1757; its caller's off-state at :1082-1083). Design record only; no code or proto change. Both prior-round HIGHs (forge taxonomy, Resolve cancellation asymmetry) re-verified intact. markdownlint 0, design-ledger-gate OK (293 rows, anchors resolve at 64364 B). Spec-impact: none. Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 5, 2026
…mbership guard (RIG-2863) Round-9 review of the gateway_credentials at-rest encryption addendum returned 0H/2M/4L — the first fold in nine rounds with no new clipped-clause defect. All seven R7 folds and both R8 mediums held; both prior HIGHs (3-mode forge taxonomy, Resolve cancellation asymmetry) intact. The two mediums are genuine frozen-contract defects the R8 admin-RPC text surfaced, both driver-foldable off the frozen C1 ruling (false-premise correction / body-vs-Tasks consistency), no Matt fork. - **M1 — "NO public proto change" is false as stated.** The record asserts it four times as a load-bearing C1 property, but the `SetServerSecret`/`DeleteServerSecret` admin RPC C1 mandates (T0) IS an additive public-proto change: `SecretsService` lives in `proto/compass/v1/compass.proto` (the public `buf.gen.yaml` codegen lane, generating the checked-in `packages/compass-client/src/gen` + `go/gen` trees), and adding the RPCs is CI-enforced — `proto/moon.yml`'s `drift` re-gens and diffs the public trees, and `go/internal/auth/classify_exhaustive_test.go` reds CI on any generated procedure not classified. Narrowed the claim to its honest bound at all five sites (:21, :249/:254, :311/:316, :947→Tasks-T0, :995): C1 needs no public proto ENUM change and leaves FetchSecrets byte-for-byte unchanged, but adds two additive `SecretsService` methods. Added the CI consequence to Tasks-T0 (`moon run compass-proto:gen` + classify both procedures or `classify_exhaustive_test` reds). Mirrored into the DL-325 ledger row (edit-before-freeze). The C1 RULING is unchanged — it genuinely removes the SERVER_ONLY delivery-kind ENUM addition, the real and correct claim. - **M2 — Tasks under-specified the user-path guard as reserved-name-only.** Tasks T0 and T2 named only the reserved-name half, contradicting the body's central F1 finding (:526-531) that a reserved-name check is INSUFFICIENT because the six configured server-secret names are arbitrary operator config (`go/cmd/compass-server/main.go:414-450`; the Linear defaults `LINEAR_FORGE_CLIENT_ID`/ `LINEAR_FORGE_CLIENT_SECRET`, serve.go:202-203) carrying no `GATEWAY_CREDENTIALS_` prefix — so the guard must be a MEMBERSHIP test. A reserved-name-only executor would ship the exact `authenticatedOpen` shadow-row hole D6 exists to close. Made both Tasks lines name both halves (membership + reserved), matching the body and the Tests bullet (already correct). - **L1** — named the dangling "the trio" antecedent inline at :232 (the declare/set/rollback trio, introduced 475 lines later). - **L2** — stated the reconcile's lock discipline explicitly (:552): same `pg_advisory_xact_lock` constant key as T2, its OWN transaction taken/released before the T2 provisioning transaction, ordering constraint only that it precede `buildForgeReadWiring`. - **L3** — re-flowed the :545-556 reconcile paragraph to the surrounding width (removed the ragged mid-clause line the R8 full-scan fold left). - **L4** — noted that `WithSystemRole`'s own doc comment (tenant_tx.go:41-47) already names the forge-notification-ack arm, so the DL-315 staleness is in the ledger prose not the code's documentation; mirrored into the DL-326 row. Both mediums grounded against tree eb5ef7a (proto public-lane + classify_exhaustive_test + six-name provenance verified). markdownlint 0, design-ledger-gate OK (293 rows, 123 headers); both DL-325/DL-326 record anchors resolve (D1/T0 headings untouched). Ledger-impact: DL-325 proto clause narrowed to ENUM-only + admin-RPC method delta; DL-326 forge-arm clause notes the doc-comment already names the arm (both edit-before-freeze). Spec-impact: none (design record only). Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 5, 2026
…pairs + F1 symmetry (RIG-2863) Round-10 review of the `gateway_credentials` at-rest encryption addendum returned 0 high / 1 medium / 4 low; all five are driver-foldable off the frozen C1 mechanism (no design fork). The single medium is a scheduling gap this record's own R9 fold created but did not schedule. ### M1 — T0 must schedule the doc-comment repairs its own changes force (medium) T0 adds a fifth `store.WithSystemRole` site (the boot reconcile) and two `adminOnly` `SecretsService` methods. Both falsify shipped code comments the moment T0 lands: `tenant_tx.go`'s "applied ONLY at the four named background-loop entrypoints" ONLY-claim (:36-38, :43-45), and five "three/3 procedures authenticatedOpen" SecretsService comments (`admin_gate.go:116-121`, `network_door.go:288-290`, `secrets_service.go:6`, `serve.go:555`, `serve.go:749-750` — all verified byte-exact at `eb5ef7a1`). This is exactly the failure DL-326 was minted to prevent, relocated from the ledger into the code. Added an explicit doc-repair deliverable to Tasks T0 and softened the body L4 clause (:620-625) to note T0 itself widens the code doc's ONLY-claim, so both surfaces are corrected in this PR chain. ### F1 symmetry + store-door parity (lows) - **Tasks T0** re-attaches SYMMETRIC to the guard *pair* (not the user-path half alone, which could license skipping the admin half) and names the admin MIRROR membership guard as an interface deliverable, at both the RPC and the `store.DeclareServerSecret` store door. - **Store bullet** (:443-452): `DeclareServerSecret` now carries the mirror of the `store.DeclareSecret` membership guard, so a `server_secrets` shadow row can never be created regardless of which writer reaches it — F1 held at BOTH doors, not only at the RPC. - **Reconcile lock discipline** (:564-569): the boot reconcile now inherits T2's bounded acquisition (`pg_try_advisory_xact_lock` in a bounded retry loop, or a session `lock_timeout`) so a contended boot fails closed with a diagnosable error rather than parking — it is the first toucher of that shared key at boot. - **Delivery enum** (:259-261): narrowed the literally-false "two-valued in all four representations" (the proto rep quotes three enum values) to "gains no new value in any of the four representations — two live delivery kinds plus the proto3 `UNSPECIFIED` zero sentinel". Both prior HIGHs (3-mode forge taxonomy silent-degradation; Resolve cancellation asymmetry) and all R7/R8/R9 folds re-confirmed intact; exhaustive citation audit found zero drifted cites. Record 68683 → 70607 B (>50KB; both DL-325/DL-326 anchors resolve). markdownlint 0, design-ledger-gate OK (293 rows, 123 headers). Ledger-impact: none (DL-325/DL-326 rows unchanged this round). Spec-impact: none. Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 5, 2026
…k + complete T0 doc-repairs (RIG-2863)
R11 review returned 0H/1M/4L. The high is a self-inflicted regression the R10 F4 fold introduced; all five findings are driver-foldable off the frozen C1 mechanism (no Matt fork — pure consistency repair of an already-frozen mechanism, completing a deliverable, and reflow).
### H1 — reconcile/store-door deadlock (R10 F4 regression)
The R10 fold made the `store.DeclareServerSecret` mirror guard **unconditional** ("shadow row can never be created regardless of which writer reaches it"), which collided head-on with T0's own boot reconcile — a writer whose defining precondition (a name still live in `secrets`) is exactly what the guard rejects. As frozen, T0 could not execute its own OQ-4 deliverable: the reconcile would fail closed on all six configured names on every existing deployment, leaving the inject-all exposure D6 exists to close open.
Fix (driver-foldable ordering repair, not the guard-free-reconcile fork): pin the reconcile's intra-transaction order to **DELETE the `secrets` row FIRST, then INSERT into `server_secrets`** within its single `WithSystemRole` transaction, so the store-door membership SELECT sees the delete already applied. Narrowed the absolute "regardless of which writer" claim at both the body store-door bullet and the Tasks T0 restatement to "no writer can create a shadow row while the name is live in `secrets`", explicitly noting the reconcile satisfies the guard by deleting first. Added a `reconcile-orders-delete-before-insert` red-green test to the T0 list (an insert-before-delete reconcile reds against the guard).
### M1 — T0 doc-repair deliverable was under-enumerated
The R10 fold scheduled five Go comment repairs but omitted two more doc comments T0's own two `adminOnly` `SecretsService` methods falsify:
- `proto/compass/v1/compass.proto:175-179` (the `SecretsService` service comment) — the worst of the set: it is the public wire contract and propagates verbatim into the drift-gated `packages/compass-client/src/gen` + `packages/compass-agent/src/gen` trees, and `moon run compass-proto:gen` regenerates the same stale text, so the drift gate does NOT catch it.
- `go/internal/auth/admin_gate.go:22-26` (the `adminOnly` type doc) — T0 adds the first non-CompassService members of this class.
Both added to the "doc-comment repairs T0's own changes require" clause, plus an opportunistic note on the pre-existing (not T0-caused) staleness at `admin_gate.go:40-41` / `classify_exhaustive_test.go:45-46`.
### Lows
- L1/L2 — reflowed two unwrapped-splice artifacts (the Tasks guard splice, the F2 lock-discipline rejoin) to the record's ~80-col convention.
- L3 — folded into M1's opportunistic note.
Record-only interdiff (+42/-15). No DECISIONS.md change (DL-325/DL-326 rows unchanged from R9). markdownlint 0 errors; design-ledger-gate OK (293 rows, 123 headers); record 72488 B (>50KB — both DL-325/DL-326 anchors resolve, D1/T0 headings untouched).
Ledger-impact: none (no DECISIONS.md edit this round).
Spec-impact: none.
Refs RIG-2863
Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 5, 2026
…reconcile requires + correct gen-lane label (RIG-2863)
Round 12 returned 0H/2M/3L. First regression-free round in twelve: the reviewer confirmed the R11 deadlock fix HOLDS at all three sites and found no new defect in the added text. Both mediums are pre-existing gaps R11's own text made newly load-bearing; folded both plus one low. L2/L3 (cosmetic reflow, no width gate) left as-is.
### M1 — the atomic single-transaction claim was not implementable through the store API the record specifies (driver-foldable false-premise correction)
R11's reconcile fix claimed "within the reconcile's single `WithSystemRole` transaction, DELETE the `secrets` row FIRST, then INSERT ... (atomic: no crash window)". But the record named the two operations as plain-signature store methods, and every such method executes through `scopedDBTX` as its OWN implicit transaction — one `pool.SendBatch` per statement (`go/internal/store/tenant_tx.go:80-92`; :64-70 states multi-statement-tx methods use `beginTenantTx` instead). So two plain calls are two transactions, the "atomic" claim is false as written, and a crash between them strands the name in NEITHER table — strictly worse than pre-fold (the heal scans only names live in `secrets`).
The store carries exactly one multi-statement-tx seam: `(*Store).WithTx(ctx, fn)` (`coordination.go:368`), which runs `fn` in one `beginTenantTx` and honors the system role via `armTx` (`tenant_tx.go:199-215`), plus the shipped `…Tx(ctx, tx, …)` method convention (`coordination.go:86/168`, `dm.go:49/97`). Naming the seam completes the already-frozen atomicity requirement — not a fork. Reconcile bullet now specifies `store.WithTx(store.WithSystemRole(ctx), …)` with tx-bound `DeleteSecretDeclarationTx`/`DeclareServerSecretTx` variants and states why the plain methods would break atomicity.
### M2 — gen-lane label corrected (the reviewer's go/gen claim did not hold against the tree)
The reviewer flagged the `SecretsService` service-comment repair as also propagating into `go/gen` (the connect-go tree). Verified against tree `eb5ef7a1`: the SERVICE comment ("User-facing secret registry writes…") has ZERO matches in either `go/gen` file — `compass.connect.go:803` is generator boilerplate and :805-812 are per-method comments T0 does not touch. So no go/gen target was missed. The real defect the finding also raised IS correct: the record mislabeled `packages/compass-agent/src/gen` as a "public" tree. Per `proto/moon.yml:35-40` it is an internal-only lane (`buf.gen.agent-ts.yaml`); only `packages/compass-client/src/gen` is public (`buf.gen.yaml`). Corrected the label; did not add the unsupported go/gen target.
### L1 — ORDERING assertion added to the T0 Plan Tests bullet
The `reconcile-orders-delete-before-insert` assertion lived only in the Tasks test list; added a matching ORDERING assertion beside RECONCILE-HEAL in the Plan Tests bullet (the prescriptive list), including the both-commit-or-both-roll-back property M1 now pins.
Record-only this round (DECISIONS.md DL-325/DL-326 unchanged). Record 72488 → 73667 B (>50KB; both anchors resolve). markdownlint 0 errors; design-ledger-gate OK (293 rows, 123 headers). Interdiff +24/-10, record only.
Ledger-impact: none. Spec-impact: none. Refs RIG-2863
Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 5, 2026
…mirror sites + fix ORDERING-assertion polarity (RIG-2863) Round 13 adversarial review of the credentials-at-rest encryption record returned 0 High / 2 Medium / 2 Low — the first regression-free round in which every finding is a driver-foldable consistency repair off the frozen mechanism, no design fork. Both mediums grounded byte-exact against the tree at `eb5ef7a1`; the two lows are one cheap citation-precision fix and one confirmed-cosmetic reflow left as-is. Record-only (DECISIONS.md DL-325/DL-326 unchanged from R9). ### M1 — the two reconcile mirror sites named a construct the R12 fold removed R12 rewrote the row-migration bullet to run the reconcile's DELETE+INSERT inside one `store.WithTx(store.WithSystemRole(ctx), …)` transaction, but left the two mirror sites (the `DeclareServerSecret` store-door bullet and the T0 Tasks restatement) saying the delete happens "inside the `WithSystemRole` transaction". After the fold there is no such construct: `store.WithSystemRole(ctx)` is a context marker (`context.WithValue`, tenant_tx.go:47-49), not a transaction, and passing it to a plain store method is exactly the two-implicit-transactions path the fold forbids. An executor implementing the store door or working the Tasks checklist from either mirror alone would write the two-transaction crash window the record now also requires a test to red on. Both mirror phrases now name the actual single-transaction construct `store.WithTx(store.WithSystemRole(ctx), …)` and the tx-bound `DeclareServerSecretTx` insert. ### M2 — the R13-added ORDERING assertion inverted the atomicity polarity The ORDERING test assertion I added in R12 stated the failure mode as the guarantee: "a crash between them strands the name in neither table". Under one transaction a crash rolls both statements back, so the name stays in `secrets` — it is stranded in neither table only under the two-transaction implementation the record forbids. The record's own corrected reconcile bullet states the correct polarity at the D6 mechanism. Fixed the assertion to "leaves the name in `secrets`, never stranded in neither table", matching the reference; the trailing "red if the DELETE and INSERT run as two transactions rather than one `store.WithTx`" parenthetical now agrees with the clause it qualifies. ### L — off-by-two citation The gen-lane classification cite `proto/moon.yml:35-40` clipped the closing of the agent-ts internal-lane clause; corrected to `:35-41`. The substance (compass-agent/src/gen internal-only via buf.gen.agent-ts.yaml; compass-client/src/gen + go/gen public via buf.gen.yaml) was already accurate. The two reflow orphans (L2/L3) are confirmed grammatically whole with no width gate; left as-is. Spec-impact: none. Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 5, 2026
…o the DeclareServerSecretTx variant the reconcile uses (RIG-2863) R14 review returned 0H/1M/2L, all driver-foldable off the frozen record. The medium is a pre-existing gap the R13 M1 fold newly exposed (not a regression): R13 correctly routed the boot reconcile through the tx-bound `DeclareServerSecretTx` variant, but the store-door membership guard was still specified only on the plain `DeclareServerSecret`. Read literally, an executor could build an unguarded `DeclareServerSecretTx` twin — bypassing F1's store door entirely and silently deleting the `reconcile-orders-delete-before-insert` tripwire (that test asserts the guard reds on the wrong-order reconcile; with no guard on the Tx path there is nothing to red). Folds (record-only; completes the already-frozen "F1 holds at BOTH doors" mechanism under the name the record already mandates — no fork, decides nothing): - **M (guard on the Tx variant)** — store-door bullet now states `DeclareServerSecret` AND its tx-bound `DeclareServerSecretTx` variant BOTH carry the mirror guard via one shared unexported check (so they can never drift); the Row-migration bullet adds that each `…Tx` variant carries the same store-door guard as its plain counterpart, so the DELETE-FIRST ordering is what admits the insert; the Tasks T0 deliverable names both variants at the store door. - **L (citation)** — `binding_cgo.go:26` → `:30` (the `C.secretspec_resolve(req)` call site; :26 is the doc-comment start). - **L (reflow orphans)** — confirmed cosmetic (no width gate, MD013 off), left as-is. Mechanical pass clean: guarded single-occurrence replaces; in-memory diff record-only +11/-6; every edited region re-read whole; backticks even, paren net 0, D1/T0 anchor headings byte-identical. markdownlint 0 errors; design-ledger-gate OK (293 rows, 123 headers). Record 73310 → 73666 B (>50KB; both DL-325/DL-326 anchors resolve). No DECISIONS.md change (DL-325/326 unchanged from R9). Ledger-impact: none (record-only; DL-325/DL-326 unchanged). Spec-impact: none. Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 5, 2026
…e to the insert side + name DeleteSecretDeclarationTx in Tasks T0 (RIG-2863) #859 R15 adversarial review returned `0H/2M/2L`. This commit folds the driver-foldable medium (a self-inflicted regression from the R14 fold) and one low; the second medium is a cross-tenant security fork routed to Matt at the merge gate (see below), not folded. ### M1 (medium, FOLDED) — R14 fold widened the guard onto the DELETE side, breaking the reconcile heal arm The R14 fold wrote the Row-migration guard clause distributively over a two-item list (`DeleteSecretDeclarationTx`/`DeclareServerSecretTx`), so "each carrying the SAME store-door membership guard" extended the guard to the delete variant too. That is wrong two ways: (1) `DeleteSecretDeclaration` carries no store-door membership guard anywhere in the record, so the delete-side claim references a guard that does not exist; (2) if an executor builds one, it rejects exactly the deletes the reconcile heal arm requires (delete a `secrets` row for a name simultaneously live in `server_secrets`, :573-583), making the `reconcile-heals-both-tables-collision` acceptance criterion unsatisfiable against the same paragraph's mandated implementation. Narrowed the clause to the insert side only — `DeclareServerSecretTx` carries the guard mirroring its plain `DeclareServerSecret` counterpart — with an explicit parenthetical that the delete side carries no membership guard (the heal arm depends on being able to delete a shadow row whose name is live in `server_secrets`), so a later fold cannot re-widen it. Driver-foldable: narrows an over-broad clause back to the R14 contract and decides nothing. ### L (low, FOLDED) — DeleteSecretDeclarationTx mandatory in Plan, absent from Tasks checklist The tx-bound `DeleteSecretDeclarationTx` is load-bearing in the Row-migration bullet but never named in the Tasks T0 deliverable list, so an executor working the checklist would not build it (no `…Tx` sibling of `DeleteSecretDeclaration` exists today, store/secrets.go:160). Added it to the T0 reconcile deliverable as a new (both-NEW) tx-bound pair. ### M2 (medium, ROUTED TO MATT — not folded) The shared store-door membership check runs a SELECT against `secrets`, which is FORCE ROW LEVEL SECURITY. The reconcile caller arms BYPASSRLS (`store.WithSystemRole`); the admin-RPC caller (`SetServerSecret`) is tenant-scoped (`compass_app`, no BYPASSRLS). An admin `SetServerSecret` for a name whose `secrets` row was declared under a different tenant runs a membership SELECT that returns zero rows, admits the insert, and creates the both-tables state F1 forbids. The fix is to arm the guard's `secrets` membership SELECT under BYPASSRLS on the admin path too — a cross-tenant BYPASSRLS widening, which this record already declares a Matt-ruled security event (:620-626 documents the identical blind spot for the reconcile). Surfaced to Matt at the merge gate with the reviewer's recommended fix; not auto-folded. ### L (low, left as-is) — reflow orphans Cosmetic, no width gate (`.markdownlint.json` is `MD013: false`). Re-confirmed grammatically whole for a fourth round. Record-only (no DECISIONS.md change; DL-325/326 unchanged). markdownlint 0 issues; design-ledger-gate OK (293 rows, 123 headers). Record 73666 → 74082 B. Ledger-impact: none. Spec-impact: none. Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 5, 2026
…time membership guard (RIG-2863) Matt-ruled design change to the RIG-2863 at-rest-encryption record, arising from the #859 R15 review's cross-tenant medium (M2): the F1 store-door membership guard ran a SELECT against `secrets` (FORCE RLS) on both the reconcile path (BYPASSRLS) and the admin RPC path (tenant-scoped `compass_app`), so an admin declaring a server secret whose name already had a `secrets` row under a different tenant would see zero rows, admit the insert, and create the both-tables state F1 forbids. Rather than patch the guard with request-path BYPASSRLS (which would breach DL-315's "never on the request path"), Matt ruled F1 into a structural partition. ### The change - **F1 is now a reserved-prefix name partition, enforced by a pure string check on both store doors.** Server-secret names carry a reserved prefix (`SERVER_` for the six forge secrets, `GATEWAY_CREDENTIALS_` for the master-key family). The admin `SetServerSecret`/`DeclareServerSecret` door REQUIRES one; the user `SetSecret`/`DeclareSecret` door REJECTS one. The two keyspaces are therefore disjoint by name — a both-tables state is UNREACHABLE in either declaration order, not healed after the fact. No cross-table membership SELECT, so no cross-tenant RLS-visibility question. **M2 is dissolved, not patched.** - **The boot reconcile is deleted.** With the partition structural, there is no both-tables state to repair: the six configured names are boot-declared under their `SERVER_`-prefixed names via an idempotent config-driven declare on the ordinary compass_app path (`server_secrets` is bucket-A, no RLS, so no BYPASSRLS, no cross-tenant read, no delete-before-insert ordering, no self-heal scan). - **DL-326 is removed from DECISIONS.md** — it existed only to record the reconcile's BYPASSRLS-allow-list widening, which no longer occurs. This record adds no cross-tenant BYPASSRLS site. DL-325's F1 description is updated from "membership guard" to the prefix partition, and its migration clause from "DB-row move" to "wiped + re-declared" (pre-production, per Matt). - **The six existing server secrets are wiped and re-declared** under their prefixed names through the new admin RPC — no in-place move, no provider-value migration (Matt: "just wipe and re-add, we don't need migration atp"). ### Sections rewritten Summary header, D6 (core F1 definition + profile-share note + retroactive-fix note), C2 alternative + named residual, T0 store-door bullet / resolver-split reconcile refs / admin-RPC guard / user-path guard / boot-declare bullet (replacing the reconcile apparatus) / Tests block, D2 master-key overwrite guard, Tasks T0 + T2, OQ-4 + OQ-5 (WITHDRAWN). A pre-existing DL-315 ledger-prose staleness (the shipped `hub.go:814` forge-notification-ack BYPASSRLS arm is absent from DL-315's four-name enumeration) surfaced during the earlier draft and is no longer this record's to fix (it no longer touches the BYPASSRLS surface); left as a separate follow-up. markdownlint 0 issues; design-ledger-gate OK (292 rows, 123 headers). Record 74082 → 68444 B. Record-only + one DECISIONS.md row edit + one row removal. Ledger-impact: DL-325 F1/migration clauses updated; DL-326 removed (its reconcile no longer exists). Spec-impact: none. Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 5, 2026
…-2863) Round-1 review of #859 found 2 high + 4 medium, all folded (no Matt ruling relitigated): - **F1 (high)** — corrected the false "user SetSecret path physically CANNOT reach the master key" claim: C1 splits the declaration registries, NOT the shared SecretSpec provider keyspace, so the reserved-name guard is now mandatory on BOTH the admin RPC and the authenticatedOpen user SetSecret/DeleteSecret path. Added the user-path red-green test and a boot key-fingerprint tripwire (T5) so a swapped provider value is detected, not silently adopted. - **F2 (high)** — replaced T0's non-executable in-migration DML row-move with a boot-time config-driven reconcile (server-secret names are per-deployment flag/env config the embedded migration cannot know), and enumerated all six names (primary + reviewer App PEM, webhook, three Linear secrets). - **F3 (medium)** — added the mandatory GRANT to compass_app/compass_system (0001's ON ALL TABLES snapshot does not cover later migrations) and a stated bucket-A RLS decision (Matt-ruled: server_secrets is deployment-global infra, no tenant_id, RLS off, added to the rls_pgtest bucketA allow-list). - **F4 (medium)** — OQ-3 resolved to D7 (Matt: api_key rows encrypted identically to OAuth rows, one seal/open path). - **F5 (medium)** — re-anchored four drifted 0001_init.sql citations (post-RIG-3106 line shift). - **F6 (medium)** — D1 column names aligned to T3/ledger (value_ciphertext/value_nonce). - **F7/F8 (low)** — added the RIG-2863 tracker ref; named the C2 residual risk concretely. Follow-up RIG-3237 filed for per-tenant at-rest isolation + gateway-topology exposure. Spec-impact: none (design record). Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 5, 2026
…(RIG-2863) Round-2 review of #859 returned 0 high / 3 medium / 3 low. The round-1 body fixes all held; the blockers were that the fix commit refreshed the record body but not the terse `## Tasks` checklist, plus one new defect the F2 fix introduced. - R2-1/R2-2 (medium): rewrite the T0 + T2 `## Tasks` checklist entries to match the fixed body — T2 no longer says the reserved-name guard is admin-RPC-only, and T0 now names the boot-time config-driven reconcile (not in-migration DML), all six server-secret names, the GRANT, and the bucketA allow-list edit. - R2-3 (medium, new defect): the F2 fix relocated the row move from a pre-policy migration to a post-policy boot reconcile, where the source `secrets` table is FORCE RLS and the ordinary tenant-scoped store path sees only the bootstrap tenant. Specify the reconcile runs under `store.WithSystemRole` (BYPASSRLS) — a fifth WithSystemRole entrypoint that extends DL-315's "only at the four named entrypoints" clause — and add the two-tenant reconcile red-green test. - R2-4 (low): add the key-swap fingerprint tripwire to the T5 checklist entry. - R2-5/R2-6 (low): correct two citation anchors added by the round-1 fixes (`rls_pgtest_test.go:544`->`:543`; `secrets_service.go:214`->`:208`). Additive review-fix commit atop the round-1 fix; no ledger delta (DL-324 is unchanged — the reconcile execution context is a T0 mechanism detail, ratified at the PR gate). Ledger-impact: none Spec-impact: none Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
force-pushed
the
compass-server/rig-2863-creds-encryption-design
branch
from
September 5, 2026 15:27
65e84a9 to
88bea70
Compare
rigel-mintaka
added a commit
that referenced
this pull request
Sep 5, 2026
…(RIG-2863) Round-5 adversarial review of PR #859 returned 2H/3M/2L; all seven verified against the tree at main `8fef2846` before folding. None is a design fork — each corrects a plan-executability or internal-consistency defect that honors an invariant the record already asserts. Additive commit atop the bookmark. - **H1 (silent forge-write capability loss):** the T0 failure-mode taxonomy was two modes ((a) hard boot fail, (b) silent Linear degradation) but a THIRD exists that the reviewer-PEM `validateForgeSecret` (serve.go:1638) hides behind. `wireForgeWriteCaller` (:587) gates on `forgeWritesEnabled` (:1583); with both App PEM names moved out of the container resolver the predicate reads both-absent, `warnPartialForgeWriteSecrets` returns silently (:1826-1827), the write caller is never mounted, and :1638 is unreachable — agent forge WRITES fail-closed fleet-wide with no boot error. Split the taxonomy into three modes, moved :1638 into mode (c), and added the missing T0 positive assertion (forge WRITE caller MOUNTED after one reconcile pass). - **H2 (uncancellable Resolve wedge):** the T2 bounded-critical-section fix prescribed a ctx timeout, but `SpecResolver.Resolve` threads ctx only into `DeclaredSecrets` (resolver.go:136); the provider round-trip is `b.Load()` (resolver.go:165), whose SDK signature carries no ctx (secretspec-go v0.15.0 secretspec.go:245) and blocks in an uncancellable FFI call. State the Set(bounded)/Resolve(unbounded) asymmetry and prescribe the mechanism that works: run the boot resolve on its own goroutine + select on the derived ctx, so the provisioner returns a bounded error and releases the xact lock while the orphaned FFI goroutine is knowingly leaked. Restated the T2 test. - **M1 (reconcile skips default-named Linear pair):** pinned the reconcile name source to `cfg.Forge.resolved()` (serve.go:232-246), not the raw flag/env layer — two Linear names carry code defaults (serve.go:202-203) applied after that layer, so a reconcile over `main.go` flags sees `""` and silently skips them, leaving them on the inject-all path. Added a T0 test. - **M2 (buildDoors resolver-fork hazard):** named the `buildDoors` fork explicitly — its single `resolver` param (serve.go:677) feeds both `buildLinearWebhookWiring` (:788, MUST repoint) and `buildNetworkServer` (:803) -> `runnerhub.NewMountedHandler` (network_door.go:313, the container FetchSecrets path that MUST NOT). So `buildDoors` takes BOTH resolvers; swapping its one argument would invert D6. - **M3 (dangling D7 reference):** DL-325 and OQ-3 cited a decision "D7" the record never defined. Promoted the api_key/OAuth uniform-treatment ruling into a real `### D7` heading in the Approach section; OQ-3 and DL-325 now resolve. - **L1:** defined the reserved prefix `GATEWAY_CREDENTIALS_` inline at its three uses (was undefined). - **L2:** caveated the header "structurally undeliverable" claim with the F1 guard dependency the D6 body already states (the split alone is necessary but not sufficient). markdownlint clean; design-ledger-gate OK (293 rows, 123 headers); no dup DL ids. Record grew to 59547 B; DL-325/DL-326 anchors already present. Ledger-impact: DL-325, DL-326 (text unchanged; D7 referent now exists) Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 5, 2026
…(RIG-2863) Round-7 review returned 0 high / 4 medium / 3 low on PR #859; all seven grounded against the tree at `eb5ef7a1` and folded additively. ### Mediums - **M1 — key-swap tripwire had no schema home.** T5 placed the non-secret key fingerprint "beside `key_version` in `server_secrets`", but `key_version` is a T3 column on `gateway_credentials` and `server_secrets` is names-only — a circular, unimplementable reference for the T0-then-T2 executor. Gave the tripwire a real home: a single-row `server_key_state` table (`key_version`/`key_fingerprint`/`fingerprint_salt`) in T0's migration, bucket-A infrastructure joining the RLS allow-list; a salted digest is not a VALUE, so the names-only invariant is untouched. Rewrote the T2 pointer and T5 placement to name it; added it to the T0 Tasks line. - **M2 — superuser citation inverted.** The record claimed `rls_pgtest_test.go:282-284` asserts the superuser-owner property "not of production", but the cited comment reads "the pgtest harness (like production) connects as a SUPERUSER owner" — the opposite — and it is a prose comment, not an assertion. Rewrote the parenthetical to match the cited text (assumption stated in two comments, not an enforced invariant). - **M3 — bounded-critical-section self-contradiction.** One sentence said "runs the boot RESOLVE/SET on its own goroutine" while the same paragraph establishes `Set` is ctx-bounded (`exec.CommandContext`, resolver.go:233) and "offloads ONLY the ctx-less provider call". Under the RESOLVE/SET reading a leaked goroutine could `Set` after the parent's Rollback, outside the released advisory lock, overwriting a key another booter already sealed rows under — the exact race the lock exists to close. Made it unambiguous: only the ctx-less `Resolve` is offloaded; the parent runs `Set` in-lock on the success branch; the offloaded goroutine performs no provider write. - **M4 — F1 membership guard one-directional.** The user path rejected a `server_secrets` name, but `SetServerSecret` carried no mirror guard against a name already live in `secrets`, and the reconcile skipped a both-tables name — the same D6-inverting end state, reachable in the opposite order and unhealed. Restated the invariant order-free ("no name ever in both tables"), made F1 symmetric (mirror guard on the admin RPC), made the reconcile self-healing (deletes the `secrets` shadow row on collision), and added the two red-green tests. ### Lows - **L1 (DECISIONS.md DL-325)** — named the symmetric F1 membership guard alongside the reserved-name guard (the reserved-name check covers only 1 of 7 names). - **L2** — added an ordering note that failure mode (c) is masked by (a) on a full no-repoint and is the partial-repoint mode. - **L3** — split the mode-(b) citation to its two real sites (`forgeSecretDeclared` at serve.go:1746-1757; its caller's off-state at :1082-1083). Design record only; no code or proto change. Both prior-round HIGHs (forge taxonomy, Resolve cancellation asymmetry) re-verified intact. markdownlint 0, design-ledger-gate OK (293 rows, anchors resolve at 64364 B). Spec-impact: none. Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 5, 2026
…mbership guard (RIG-2863) Round-9 review of the gateway_credentials at-rest encryption addendum returned 0H/2M/4L — the first fold in nine rounds with no new clipped-clause defect. All seven R7 folds and both R8 mediums held; both prior HIGHs (3-mode forge taxonomy, Resolve cancellation asymmetry) intact. The two mediums are genuine frozen-contract defects the R8 admin-RPC text surfaced, both driver-foldable off the frozen C1 ruling (false-premise correction / body-vs-Tasks consistency), no Matt fork. - **M1 — "NO public proto change" is false as stated.** The record asserts it four times as a load-bearing C1 property, but the `SetServerSecret`/`DeleteServerSecret` admin RPC C1 mandates (T0) IS an additive public-proto change: `SecretsService` lives in `proto/compass/v1/compass.proto` (the public `buf.gen.yaml` codegen lane, generating the checked-in `packages/compass-client/src/gen` + `go/gen` trees), and adding the RPCs is CI-enforced — `proto/moon.yml`'s `drift` re-gens and diffs the public trees, and `go/internal/auth/classify_exhaustive_test.go` reds CI on any generated procedure not classified. Narrowed the claim to its honest bound at all five sites (:21, :249/:254, :311/:316, :947→Tasks-T0, :995): C1 needs no public proto ENUM change and leaves FetchSecrets byte-for-byte unchanged, but adds two additive `SecretsService` methods. Added the CI consequence to Tasks-T0 (`moon run compass-proto:gen` + classify both procedures or `classify_exhaustive_test` reds). Mirrored into the DL-325 ledger row (edit-before-freeze). The C1 RULING is unchanged — it genuinely removes the SERVER_ONLY delivery-kind ENUM addition, the real and correct claim. - **M2 — Tasks under-specified the user-path guard as reserved-name-only.** Tasks T0 and T2 named only the reserved-name half, contradicting the body's central F1 finding (:526-531) that a reserved-name check is INSUFFICIENT because the six configured server-secret names are arbitrary operator config (`go/cmd/compass-server/main.go:414-450`; the Linear defaults `LINEAR_FORGE_CLIENT_ID`/ `LINEAR_FORGE_CLIENT_SECRET`, serve.go:202-203) carrying no `GATEWAY_CREDENTIALS_` prefix — so the guard must be a MEMBERSHIP test. A reserved-name-only executor would ship the exact `authenticatedOpen` shadow-row hole D6 exists to close. Made both Tasks lines name both halves (membership + reserved), matching the body and the Tests bullet (already correct). - **L1** — named the dangling "the trio" antecedent inline at :232 (the declare/set/rollback trio, introduced 475 lines later). - **L2** — stated the reconcile's lock discipline explicitly (:552): same `pg_advisory_xact_lock` constant key as T2, its OWN transaction taken/released before the T2 provisioning transaction, ordering constraint only that it precede `buildForgeReadWiring`. - **L3** — re-flowed the :545-556 reconcile paragraph to the surrounding width (removed the ragged mid-clause line the R8 full-scan fold left). - **L4** — noted that `WithSystemRole`'s own doc comment (tenant_tx.go:41-47) already names the forge-notification-ack arm, so the DL-315 staleness is in the ledger prose not the code's documentation; mirrored into the DL-326 row. Both mediums grounded against tree eb5ef7a (proto public-lane + classify_exhaustive_test + six-name provenance verified). markdownlint 0, design-ledger-gate OK (293 rows, 123 headers); both DL-325/DL-326 record anchors resolve (D1/T0 headings untouched). Ledger-impact: DL-325 proto clause narrowed to ENUM-only + admin-RPC method delta; DL-326 forge-arm clause notes the doc-comment already names the arm (both edit-before-freeze). Spec-impact: none (design record only). Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 5, 2026
…pairs + F1 symmetry (RIG-2863) Round-10 review of the `gateway_credentials` at-rest encryption addendum returned 0 high / 1 medium / 4 low; all five are driver-foldable off the frozen C1 mechanism (no design fork). The single medium is a scheduling gap this record's own R9 fold created but did not schedule. ### M1 — T0 must schedule the doc-comment repairs its own changes force (medium) T0 adds a fifth `store.WithSystemRole` site (the boot reconcile) and two `adminOnly` `SecretsService` methods. Both falsify shipped code comments the moment T0 lands: `tenant_tx.go`'s "applied ONLY at the four named background-loop entrypoints" ONLY-claim (:36-38, :43-45), and five "three/3 procedures authenticatedOpen" SecretsService comments (`admin_gate.go:116-121`, `network_door.go:288-290`, `secrets_service.go:6`, `serve.go:555`, `serve.go:749-750` — all verified byte-exact at `eb5ef7a1`). This is exactly the failure DL-326 was minted to prevent, relocated from the ledger into the code. Added an explicit doc-repair deliverable to Tasks T0 and softened the body L4 clause (:620-625) to note T0 itself widens the code doc's ONLY-claim, so both surfaces are corrected in this PR chain. ### F1 symmetry + store-door parity (lows) - **Tasks T0** re-attaches SYMMETRIC to the guard *pair* (not the user-path half alone, which could license skipping the admin half) and names the admin MIRROR membership guard as an interface deliverable, at both the RPC and the `store.DeclareServerSecret` store door. - **Store bullet** (:443-452): `DeclareServerSecret` now carries the mirror of the `store.DeclareSecret` membership guard, so a `server_secrets` shadow row can never be created regardless of which writer reaches it — F1 held at BOTH doors, not only at the RPC. - **Reconcile lock discipline** (:564-569): the boot reconcile now inherits T2's bounded acquisition (`pg_try_advisory_xact_lock` in a bounded retry loop, or a session `lock_timeout`) so a contended boot fails closed with a diagnosable error rather than parking — it is the first toucher of that shared key at boot. - **Delivery enum** (:259-261): narrowed the literally-false "two-valued in all four representations" (the proto rep quotes three enum values) to "gains no new value in any of the four representations — two live delivery kinds plus the proto3 `UNSPECIFIED` zero sentinel". Both prior HIGHs (3-mode forge taxonomy silent-degradation; Resolve cancellation asymmetry) and all R7/R8/R9 folds re-confirmed intact; exhaustive citation audit found zero drifted cites. Record 68683 → 70607 B (>50KB; both DL-325/DL-326 anchors resolve). markdownlint 0, design-ledger-gate OK (293 rows, 123 headers). Ledger-impact: none (DL-325/DL-326 rows unchanged this round). Spec-impact: none. Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 5, 2026
…k + complete T0 doc-repairs (RIG-2863)
R11 review returned 0H/1M/4L. The high is a self-inflicted regression the R10 F4 fold introduced; all five findings are driver-foldable off the frozen C1 mechanism (no Matt fork — pure consistency repair of an already-frozen mechanism, completing a deliverable, and reflow).
### H1 — reconcile/store-door deadlock (R10 F4 regression)
The R10 fold made the `store.DeclareServerSecret` mirror guard **unconditional** ("shadow row can never be created regardless of which writer reaches it"), which collided head-on with T0's own boot reconcile — a writer whose defining precondition (a name still live in `secrets`) is exactly what the guard rejects. As frozen, T0 could not execute its own OQ-4 deliverable: the reconcile would fail closed on all six configured names on every existing deployment, leaving the inject-all exposure D6 exists to close open.
Fix (driver-foldable ordering repair, not the guard-free-reconcile fork): pin the reconcile's intra-transaction order to **DELETE the `secrets` row FIRST, then INSERT into `server_secrets`** within its single `WithSystemRole` transaction, so the store-door membership SELECT sees the delete already applied. Narrowed the absolute "regardless of which writer" claim at both the body store-door bullet and the Tasks T0 restatement to "no writer can create a shadow row while the name is live in `secrets`", explicitly noting the reconcile satisfies the guard by deleting first. Added a `reconcile-orders-delete-before-insert` red-green test to the T0 list (an insert-before-delete reconcile reds against the guard).
### M1 — T0 doc-repair deliverable was under-enumerated
The R10 fold scheduled five Go comment repairs but omitted two more doc comments T0's own two `adminOnly` `SecretsService` methods falsify:
- `proto/compass/v1/compass.proto:175-179` (the `SecretsService` service comment) — the worst of the set: it is the public wire contract and propagates verbatim into the drift-gated `packages/compass-client/src/gen` + `packages/compass-agent/src/gen` trees, and `moon run compass-proto:gen` regenerates the same stale text, so the drift gate does NOT catch it.
- `go/internal/auth/admin_gate.go:22-26` (the `adminOnly` type doc) — T0 adds the first non-CompassService members of this class.
Both added to the "doc-comment repairs T0's own changes require" clause, plus an opportunistic note on the pre-existing (not T0-caused) staleness at `admin_gate.go:40-41` / `classify_exhaustive_test.go:45-46`.
### Lows
- L1/L2 — reflowed two unwrapped-splice artifacts (the Tasks guard splice, the F2 lock-discipline rejoin) to the record's ~80-col convention.
- L3 — folded into M1's opportunistic note.
Record-only interdiff (+42/-15). No DECISIONS.md change (DL-325/DL-326 rows unchanged from R9). markdownlint 0 errors; design-ledger-gate OK (293 rows, 123 headers); record 72488 B (>50KB — both DL-325/DL-326 anchors resolve, D1/T0 headings untouched).
Ledger-impact: none (no DECISIONS.md edit this round).
Spec-impact: none.
Refs RIG-2863
Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 5, 2026
…reconcile requires + correct gen-lane label (RIG-2863)
Round 12 returned 0H/2M/3L. First regression-free round in twelve: the reviewer confirmed the R11 deadlock fix HOLDS at all three sites and found no new defect in the added text. Both mediums are pre-existing gaps R11's own text made newly load-bearing; folded both plus one low. L2/L3 (cosmetic reflow, no width gate) left as-is.
### M1 — the atomic single-transaction claim was not implementable through the store API the record specifies (driver-foldable false-premise correction)
R11's reconcile fix claimed "within the reconcile's single `WithSystemRole` transaction, DELETE the `secrets` row FIRST, then INSERT ... (atomic: no crash window)". But the record named the two operations as plain-signature store methods, and every such method executes through `scopedDBTX` as its OWN implicit transaction — one `pool.SendBatch` per statement (`go/internal/store/tenant_tx.go:80-92`; :64-70 states multi-statement-tx methods use `beginTenantTx` instead). So two plain calls are two transactions, the "atomic" claim is false as written, and a crash between them strands the name in NEITHER table — strictly worse than pre-fold (the heal scans only names live in `secrets`).
The store carries exactly one multi-statement-tx seam: `(*Store).WithTx(ctx, fn)` (`coordination.go:368`), which runs `fn` in one `beginTenantTx` and honors the system role via `armTx` (`tenant_tx.go:199-215`), plus the shipped `…Tx(ctx, tx, …)` method convention (`coordination.go:86/168`, `dm.go:49/97`). Naming the seam completes the already-frozen atomicity requirement — not a fork. Reconcile bullet now specifies `store.WithTx(store.WithSystemRole(ctx), …)` with tx-bound `DeleteSecretDeclarationTx`/`DeclareServerSecretTx` variants and states why the plain methods would break atomicity.
### M2 — gen-lane label corrected (the reviewer's go/gen claim did not hold against the tree)
The reviewer flagged the `SecretsService` service-comment repair as also propagating into `go/gen` (the connect-go tree). Verified against tree `eb5ef7a1`: the SERVICE comment ("User-facing secret registry writes…") has ZERO matches in either `go/gen` file — `compass.connect.go:803` is generator boilerplate and :805-812 are per-method comments T0 does not touch. So no go/gen target was missed. The real defect the finding also raised IS correct: the record mislabeled `packages/compass-agent/src/gen` as a "public" tree. Per `proto/moon.yml:35-40` it is an internal-only lane (`buf.gen.agent-ts.yaml`); only `packages/compass-client/src/gen` is public (`buf.gen.yaml`). Corrected the label; did not add the unsupported go/gen target.
### L1 — ORDERING assertion added to the T0 Plan Tests bullet
The `reconcile-orders-delete-before-insert` assertion lived only in the Tasks test list; added a matching ORDERING assertion beside RECONCILE-HEAL in the Plan Tests bullet (the prescriptive list), including the both-commit-or-both-roll-back property M1 now pins.
Record-only this round (DECISIONS.md DL-325/DL-326 unchanged). Record 72488 → 73667 B (>50KB; both anchors resolve). markdownlint 0 errors; design-ledger-gate OK (293 rows, 123 headers). Interdiff +24/-10, record only.
Ledger-impact: none. Spec-impact: none. Refs RIG-2863
Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 5, 2026
…mirror sites + fix ORDERING-assertion polarity (RIG-2863) Round 13 adversarial review of the credentials-at-rest encryption record returned 0 High / 2 Medium / 2 Low — the first regression-free round in which every finding is a driver-foldable consistency repair off the frozen mechanism, no design fork. Both mediums grounded byte-exact against the tree at `eb5ef7a1`; the two lows are one cheap citation-precision fix and one confirmed-cosmetic reflow left as-is. Record-only (DECISIONS.md DL-325/DL-326 unchanged from R9). ### M1 — the two reconcile mirror sites named a construct the R12 fold removed R12 rewrote the row-migration bullet to run the reconcile's DELETE+INSERT inside one `store.WithTx(store.WithSystemRole(ctx), …)` transaction, but left the two mirror sites (the `DeclareServerSecret` store-door bullet and the T0 Tasks restatement) saying the delete happens "inside the `WithSystemRole` transaction". After the fold there is no such construct: `store.WithSystemRole(ctx)` is a context marker (`context.WithValue`, tenant_tx.go:47-49), not a transaction, and passing it to a plain store method is exactly the two-implicit-transactions path the fold forbids. An executor implementing the store door or working the Tasks checklist from either mirror alone would write the two-transaction crash window the record now also requires a test to red on. Both mirror phrases now name the actual single-transaction construct `store.WithTx(store.WithSystemRole(ctx), …)` and the tx-bound `DeclareServerSecretTx` insert. ### M2 — the R13-added ORDERING assertion inverted the atomicity polarity The ORDERING test assertion I added in R12 stated the failure mode as the guarantee: "a crash between them strands the name in neither table". Under one transaction a crash rolls both statements back, so the name stays in `secrets` — it is stranded in neither table only under the two-transaction implementation the record forbids. The record's own corrected reconcile bullet states the correct polarity at the D6 mechanism. Fixed the assertion to "leaves the name in `secrets`, never stranded in neither table", matching the reference; the trailing "red if the DELETE and INSERT run as two transactions rather than one `store.WithTx`" parenthetical now agrees with the clause it qualifies. ### L — off-by-two citation The gen-lane classification cite `proto/moon.yml:35-40` clipped the closing of the agent-ts internal-lane clause; corrected to `:35-41`. The substance (compass-agent/src/gen internal-only via buf.gen.agent-ts.yaml; compass-client/src/gen + go/gen public via buf.gen.yaml) was already accurate. The two reflow orphans (L2/L3) are confirmed grammatically whole with no width gate; left as-is. Spec-impact: none. Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 5, 2026
…o the DeclareServerSecretTx variant the reconcile uses (RIG-2863) R14 review returned 0H/1M/2L, all driver-foldable off the frozen record. The medium is a pre-existing gap the R13 M1 fold newly exposed (not a regression): R13 correctly routed the boot reconcile through the tx-bound `DeclareServerSecretTx` variant, but the store-door membership guard was still specified only on the plain `DeclareServerSecret`. Read literally, an executor could build an unguarded `DeclareServerSecretTx` twin — bypassing F1's store door entirely and silently deleting the `reconcile-orders-delete-before-insert` tripwire (that test asserts the guard reds on the wrong-order reconcile; with no guard on the Tx path there is nothing to red). Folds (record-only; completes the already-frozen "F1 holds at BOTH doors" mechanism under the name the record already mandates — no fork, decides nothing): - **M (guard on the Tx variant)** — store-door bullet now states `DeclareServerSecret` AND its tx-bound `DeclareServerSecretTx` variant BOTH carry the mirror guard via one shared unexported check (so they can never drift); the Row-migration bullet adds that each `…Tx` variant carries the same store-door guard as its plain counterpart, so the DELETE-FIRST ordering is what admits the insert; the Tasks T0 deliverable names both variants at the store door. - **L (citation)** — `binding_cgo.go:26` → `:30` (the `C.secretspec_resolve(req)` call site; :26 is the doc-comment start). - **L (reflow orphans)** — confirmed cosmetic (no width gate, MD013 off), left as-is. Mechanical pass clean: guarded single-occurrence replaces; in-memory diff record-only +11/-6; every edited region re-read whole; backticks even, paren net 0, D1/T0 anchor headings byte-identical. markdownlint 0 errors; design-ledger-gate OK (293 rows, 123 headers). Record 73310 → 73666 B (>50KB; both DL-325/DL-326 anchors resolve). No DECISIONS.md change (DL-325/326 unchanged from R9). Ledger-impact: none (record-only; DL-325/DL-326 unchanged). Spec-impact: none. Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 5, 2026
…e to the insert side + name DeleteSecretDeclarationTx in Tasks T0 (RIG-2863) #859 R15 adversarial review returned `0H/2M/2L`. This commit folds the driver-foldable medium (a self-inflicted regression from the R14 fold) and one low; the second medium is a cross-tenant security fork routed to Matt at the merge gate (see below), not folded. ### M1 (medium, FOLDED) — R14 fold widened the guard onto the DELETE side, breaking the reconcile heal arm The R14 fold wrote the Row-migration guard clause distributively over a two-item list (`DeleteSecretDeclarationTx`/`DeclareServerSecretTx`), so "each carrying the SAME store-door membership guard" extended the guard to the delete variant too. That is wrong two ways: (1) `DeleteSecretDeclaration` carries no store-door membership guard anywhere in the record, so the delete-side claim references a guard that does not exist; (2) if an executor builds one, it rejects exactly the deletes the reconcile heal arm requires (delete a `secrets` row for a name simultaneously live in `server_secrets`, :573-583), making the `reconcile-heals-both-tables-collision` acceptance criterion unsatisfiable against the same paragraph's mandated implementation. Narrowed the clause to the insert side only — `DeclareServerSecretTx` carries the guard mirroring its plain `DeclareServerSecret` counterpart — with an explicit parenthetical that the delete side carries no membership guard (the heal arm depends on being able to delete a shadow row whose name is live in `server_secrets`), so a later fold cannot re-widen it. Driver-foldable: narrows an over-broad clause back to the R14 contract and decides nothing. ### L (low, FOLDED) — DeleteSecretDeclarationTx mandatory in Plan, absent from Tasks checklist The tx-bound `DeleteSecretDeclarationTx` is load-bearing in the Row-migration bullet but never named in the Tasks T0 deliverable list, so an executor working the checklist would not build it (no `…Tx` sibling of `DeleteSecretDeclaration` exists today, store/secrets.go:160). Added it to the T0 reconcile deliverable as a new (both-NEW) tx-bound pair. ### M2 (medium, ROUTED TO MATT — not folded) The shared store-door membership check runs a SELECT against `secrets`, which is FORCE ROW LEVEL SECURITY. The reconcile caller arms BYPASSRLS (`store.WithSystemRole`); the admin-RPC caller (`SetServerSecret`) is tenant-scoped (`compass_app`, no BYPASSRLS). An admin `SetServerSecret` for a name whose `secrets` row was declared under a different tenant runs a membership SELECT that returns zero rows, admits the insert, and creates the both-tables state F1 forbids. The fix is to arm the guard's `secrets` membership SELECT under BYPASSRLS on the admin path too — a cross-tenant BYPASSRLS widening, which this record already declares a Matt-ruled security event (:620-626 documents the identical blind spot for the reconcile). Surfaced to Matt at the merge gate with the reviewer's recommended fix; not auto-folded. ### L (low, left as-is) — reflow orphans Cosmetic, no width gate (`.markdownlint.json` is `MD013: false`). Re-confirmed grammatically whole for a fourth round. Record-only (no DECISIONS.md change; DL-325/326 unchanged). markdownlint 0 issues; design-ledger-gate OK (293 rows, 123 headers). Record 73666 → 74082 B. Ledger-impact: none. Spec-impact: none. Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 5, 2026
…time membership guard (RIG-2863) Matt-ruled design change to the RIG-2863 at-rest-encryption record, arising from the #859 R15 review's cross-tenant medium (M2): the F1 store-door membership guard ran a SELECT against `secrets` (FORCE RLS) on both the reconcile path (BYPASSRLS) and the admin RPC path (tenant-scoped `compass_app`), so an admin declaring a server secret whose name already had a `secrets` row under a different tenant would see zero rows, admit the insert, and create the both-tables state F1 forbids. Rather than patch the guard with request-path BYPASSRLS (which would breach DL-315's "never on the request path"), Matt ruled F1 into a structural partition. ### The change - **F1 is now a reserved-prefix name partition, enforced by a pure string check on both store doors.** Server-secret names carry a reserved prefix (`SERVER_` for the six forge secrets, `GATEWAY_CREDENTIALS_` for the master-key family). The admin `SetServerSecret`/`DeclareServerSecret` door REQUIRES one; the user `SetSecret`/`DeclareSecret` door REJECTS one. The two keyspaces are therefore disjoint by name — a both-tables state is UNREACHABLE in either declaration order, not healed after the fact. No cross-table membership SELECT, so no cross-tenant RLS-visibility question. **M2 is dissolved, not patched.** - **The boot reconcile is deleted.** With the partition structural, there is no both-tables state to repair: the six configured names are boot-declared under their `SERVER_`-prefixed names via an idempotent config-driven declare on the ordinary compass_app path (`server_secrets` is bucket-A, no RLS, so no BYPASSRLS, no cross-tenant read, no delete-before-insert ordering, no self-heal scan). - **DL-326 is removed from DECISIONS.md** — it existed only to record the reconcile's BYPASSRLS-allow-list widening, which no longer occurs. This record adds no cross-tenant BYPASSRLS site. DL-325's F1 description is updated from "membership guard" to the prefix partition, and its migration clause from "DB-row move" to "wiped + re-declared" (pre-production, per Matt). - **The six existing server secrets are wiped and re-declared** under their prefixed names through the new admin RPC — no in-place move, no provider-value migration (Matt: "just wipe and re-add, we don't need migration atp"). ### Sections rewritten Summary header, D6 (core F1 definition + profile-share note + retroactive-fix note), C2 alternative + named residual, T0 store-door bullet / resolver-split reconcile refs / admin-RPC guard / user-path guard / boot-declare bullet (replacing the reconcile apparatus) / Tests block, D2 master-key overwrite guard, Tasks T0 + T2, OQ-4 + OQ-5 (WITHDRAWN). A pre-existing DL-315 ledger-prose staleness (the shipped `hub.go:814` forge-notification-ack BYPASSRLS arm is absent from DL-315's four-name enumeration) surfaced during the earlier draft and is no longer this record's to fix (it no longer touches the BYPASSRLS surface); left as a separate follow-up. markdownlint 0 issues; design-ledger-gate OK (292 rows, 123 headers). Record 74082 → 68444 B. Record-only + one DECISIONS.md row edit + one row removal. Ledger-impact: DL-325 F1/migration clauses updated; DL-326 removed (its reconcile no longer exists). Spec-impact: none. Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
…et providers (RIG-2863) Design record for encrypting the `gateway_credentials` value payload at rest (AES-256-GCM envelope, application-side) plus a physically separate `server_secrets` store, resolved through the review loop (R1–R17) and a Matt-ruled two-layer secret-provider architecture. (History flattened onto current main after a concurrent-merge DL-number collision forced a renumber; the folded record is the reviewed content.) ### Two-layer secret-provider architecture (Matt-ruled) Server-boot creds (the six `SERVER_` forge secrets + the `GATEWAY_CREDENTIALS_` master key) are custodied in an operator-chosen WRITABLE SecretSpec provider, resolved at boot — NOT provisioned through a running-server RPC (which was a boot chicken-and-egg: `validateForgeSecret`'s hard-fail on an absent name at `buildForgeReadWiring` → `failStartup`, serve.go:567-569, runs before the RPC doors are built). Self-hosted default `age://` (writable, encrypted-at-rest, headless/no D-Bus); pluggable per deployment to a cloud store (`awssm`/`awsps`/`aac`/`akv`) or Vault/OpenBao where present. Values are present at first boot from the provider; the hard-fail stays but is now a clean static deploy-time error. The user/agent write-back-cred keyring→provider default migration is a separate follow-up. ### Review-loop findings (R17 final fold) - **serverSecretName() seam as an INVARIANT** — every argument compared against a resolved `ResolvedSecret.Name` is prefix-wrapped (including `secretDeclared` in `forgeWriteAppsConfigured` serve.go:272 and `forgeSecretDeclared` serve.go:1078); every log/error/operator-config string is not. - **Idempotence at the RPC layer** — `SetServerSecret` tolerates ErrConflict and rewrites the provider value (secrets_service.go:117-118); store `DeclareServerSecret` mirrors the non-idempotent `DeclareSecret`. - **`compass server-secret` CLI** — a T0 deliverable for runtime rotation/management, stdin-only value. ### Ledger New DL-328 (encryption + server-secret partition + provider custody), Record link anchored to the D1 heading. Renumbered from a transient DL-326 after concurrent merges took DL-325 (RIG-3070), DL-326 (RIG-3170 P2 volume); the sibling SubjectService PR holds DL-327. Ledger-impact: adds DL-328 (gateway-credentials at-rest encryption). Spec-impact: none. Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
force-pushed
the
compass-server/rig-2863-creds-encryption-design
branch
from
September 5, 2026 19:55
9726837 to
e9869f1
Compare
…boot idempotence (RIG-2863) Addresses the R18 adversarial review of the gateway-credentials at-rest encryption record (1H/3M/3L, all driver-foldable — zero forks). ### High - **age:// unresolvable on the pinned dependency, no version prerequisite stated (H-1).** The two-layer boot model makes `age://` the load-bearing self-hosted default for all seven server-secret values, but the repo pins `secretspec-go` at v0.15.0 (go/go.mod:22) where `age` (a 0.17+ build-feature provider) does not resolve and T2's master-key write-back has no writable target. F2 + T0 now state the gating dependency prerequisite (bump to >= 0.17 with the `age` build feature, a separate Matt-ruled PR) and name the wiring deliverable: T0 constructs the SERVER resolver `WithProvider(<URI>)` (resolver.go:75) off a new flag/env, since serve.go:528 today builds the resolver with no provider option. Mirrored into the DL-328 row. ### Medium - **Stale-model survivors in D6 (M-1).** Two D6 paragraphs (:258-261, :300-301) still named the admin RPC as the provisioning path for the six, contradicting the rewritten F2/OQ-4/DL-328 provider model. Rewritten onto boot-declare + provider-populate-before-boot; RPC/CLI scoped to rotation. - **Boot-declare ErrConflict disposition undefined (M-2).** The boot declare of the six names runs every boot; the store method is non-idempotent, so a second boot would surface ErrConflict. F2 now states the boot declare is idempotent by construction (tolerates ErrConflict as the already-declared no-op), plus a second-boot Tests assertion. - **`env` listed as a valid provider for write-back values (M-3).** The value- custody paragraph asserted a writable-provider invariant then listed `env` (read-only in secretspec) as an acceptable population surface. `env` dropped, scoped as read-only-insufficient for the server-minted master key. ### Low - L-1: Tasks citation for `forgeWriteAppsConfigured` widened serve.go:272 → :272-273 (both primary and reviewer arms). - L-2: the `Builder.Load()` no-ctx SDK fact flagged to re-verify after the >= 0.17 bump. - L-3: the Tests six-names assertion rescoped from RPC-provisioning to rotation. Anomalously long fold-new lines reflowed to the surrounding prose norm. Gates green: root:markdownlint 0 errors; design-ledger-gate OK (294 rows, 124 record headers). Ledger-impact: DL-328 row gains the age:// version-prerequisite clause; no new DL number. Spec-impact: none. Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
…ix correction (RIG-2863) Folds the R19 review's determinate findings on the gateway-credentials at-rest-encryption record (additive atop the R18 fold; the one non-foldable finding, the shared-provider-keyspace premise vs per-instance provider URI, is routed to Matt separately). - M-1: the `age://` version prerequisite now names BOTH closures the two code paths shell separately — the `secretspec-go` SDK module for the READ path (`b.Load()`, resolver.go:165) AND the `secretspec` CLI binary the WRITE path runs (`resolver.Set` shells `r.cli`, resolver.go:233/91), pinned via `WithCLI` (resolver.go:81) into the Server closure so read/write cannot drift to different provider capabilities. Applied in F2, T0 Tasks, and the DL-328 mirror clause; adds a T0 red-green assertion that the write path resolves `age://` against the staged CLI, not only the SDK. - M-3: corrects the false "writable providers are dotenv/keyring" claim against the secretspec matrix (keyring, dotenv, pass, gopass 0.15+, awssm, akv, vault are all Write-capable at the current pin); re-scopes the bump as gating the self-hosted `age://` default path ONLY, not T0/T2 wholesale — a cloud/Vault deployment needs no bump. - L-1: normalizes four `secrets_service.go` ErrConflict-arm citations to :116-117 (the case line + its comment; :118 is the next case). - L-2: appends the `>= 0.17` gate qualifier to OQ-4's resolved `age://` default. - L-3: annotates the cloud-store alternatives with their version floors (`awsps` 0.18+, `aac` 0.20+) so the list stays consistent with the new floor. Ledger-impact: DL-328 mirror clause widened to name both closures. Spec-impact: none (design record only). Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
…tance URI (RIG-2863) Folds R19's one non-foldable finding (M-2), Matt-ruled via `ask`: the two-layer provider wiring gives the SERVER resolver its own URI, which left the record's shared-provider-keyspace premise (justifying the F1 user-path guard and the D2 boot read-back) unreconciled. Matt's ruling: default shared, operator may optionally opt Layer B onto a different provider. - F2 WIRING SEAM: by default one operator URI configures BOTH resolver instances (the SERVER resolver and the serve.go:528 container/user resolver), so the keyspace is shared by construction; an operator MAY optionally point the Layer-B resolver at a different provider via a second flag/env, isolating the two keyspaces. - Premise (:255), F1 guard rationale, and D2 read-back rationale: restated as "shared by default" rather than an unconditional invariant. F1 is now stated as a declaration-layer NAME partition independent of the shared-keyspace default; D2's read-back is stated as a master-key integrity control retained even under a Layer-B split (defends against an operator's out-of-band overwrite and a swapped provider file regardless of keyspace sharing). - C2 alternative note: the optional Layer-B split already delivers provider-keyspace isolation for the user SetSecret path when opted into; C2 would make that isolation structural/unconditional (a dedicated profile). Ledger-impact: none (DL-328 already frames F1 as a declaration-layer partition, consistent with the reconcile). Spec-impact: none (design record only). Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
…G-2863)
R20 review fold on the gateway-credentials at-rest encryption record.
- M-1: reconcile the T2 §Master-key overwrite guard block onto the
default-shared/optional-split model the M-2 reconcile applied at the T0
guard block (:596-598) and F2 seam. The T2 block still asserted an
unconditional shared provider keyspace ("but NOT the provider keyspace",
"against the SHARED keyspace"), contradicting the C2 note the same fold
wrote; qualified both as default-shared and added the retained-under-split
clause (F1 is a declaration-layer name partition independent of provider).
- M-2: OpenBao does not qualify for the no-bump enumeration. `openbao://` is
a 0.17+ provider (0.16 routes it through `vault`), so an OpenBao deployment
rides the same secretspec bump as `age://`. Moved it out of the current-pin
Write-capable list in F2 (:633) and T0 Tasks (:1147); the target-state
mentions (:622, :1075, :1226) are unaffected.
- L: softened the DL-328 premise clause to "not (by default) the shared
provider keyspace" so the row does not read as ruling out the operator
Layer-B split the body now permits.
Ledger-impact: DL-328 premise clause softened; no decision/status/link change.
Spec-impact: none. Refs RIG-2863
Co-authored-by: Matt Wilkinson <matt@rigel.build>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addendum to the frozen
compass-server-llm-gatewayrecord: encryption-at-restfor the
gateway_credentialsvalue store, master-key custody, and a physicallyseparate
server_secretsstore for server-only secrets.Decisions (all Matt-ruled):
key_version columns, no plaintext value column ever; fresh 96-bit nonce per
write (2^32-per-key bound quantified against the OAuth-refresh write rate).
configured SecretSpec provider under a Postgres advisory lock with an
every-boot read-back verify; key never touches the DB.
key_versioncolumn from day one (cheap now, expensive retrofit).MarshalJSON), never the RPC serialization type.
server_secretsstore — a names-only table + a SECOND SpecResolver instanceover it (shared profile) + an admin-gated SetServerSecret/DeleteServerSecret
RPC. The delivery boundary is the resolver's manifest, so a server secret is
STRUCTURALLY undeliverable to agent containers (default-closed by
construction). Removes the public proto change the earlier
SERVER_ONLY-delivery-kind fold required; the container FetchSecrets path is
byte-for-byte unchanged.
The PEM/webhook/Linear server secrets — server_only by convention only today,
riding the inject-all path into every agent container — move into
server_secretsin this PR chain (a DB-row move, provider values untouched),closing that pre-existing exposure (OQ-4, Matt-ruled: same PR chain).
Tasks: T0 server_secrets store (prerequisite) → T1 envelope pkg → T2 boot
provision seam → T3 ciphertext columns → T4 seal/open wiring → T5 tripwire +
redaction assertions.
Open sub-fork flagged for review: the server-provisioned master key's
declared_byFK is specced NULLABLE (NULL = server-provisioned; honestprovenance, no boot-ordering coupling) rather than attributed to the
bootstrap-admin account or an @Compass sentinel — a one-line T0 change either
way with no downstream effect.
Ledger: DL-324 (Storage).
Spec-impact: adds
server_secretstable + SetServerSecret/DeleteServerSecretadmin RPC + gateway_credentials ciphertext columns to the RIG-2863 build.
Refs RIG-2863
Co-authored-by: Matt Wilkinson matt@rigel.build